-
Couldn't load subscription status.
- Fork 20
Update microgrid client #1230
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update microgrid client #1230
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR updates the microgrid client integration to the latest version by changing import paths and filtering out StreamEvents where necessary. It also updates dependency references and release notes accordingly.
- Updated import locations for ComponentId across multiple modules.
- Introduced filtering of stream events in various data stream receivers.
- Adjusted dependency declaration in pyproject.toml and updated release notes.
Reviewed Changes
Copilot reviewed 69 out of 69 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/frequenz/sdk/microgrid/_power_distributing/request.py | Updated import for ComponentId from the new location. |
| src/frequenz/sdk/microgrid/_power_distributing/power_distributing.py | Adjusted ComponentId import and removed unused import items. |
| src/frequenz/sdk/microgrid/_power_distributing/_distribution_algorithm/_battery_distribution_algorithm.py | Updated ComponentId import with consistent module path. |
| src/frequenz/sdk/microgrid/_power_distributing/_component_status/_pv_inverter_status_tracker.py | Revised ComponentId import and added stream filtering for InverterData. |
| src/frequenz/sdk/microgrid/_power_distributing/_component_status/_ev_charger_status_tracker.py | Updated imports and wrapped EVCharger receiver with filter_stream_events. |
| src/frequenz/sdk/microgrid/_power_distributing/_component_status/_component_status.py | Updated ComponentId import. |
| src/frequenz/sdk/microgrid/_power_distributing/_component_status/_battery_status_tracker.py | Applied filtering for battery and inverter receivers. |
| src/frequenz/sdk/microgrid/_power_distributing/_component_pool_status_tracker.py | Updated ComponentId import. |
| src/frequenz/sdk/microgrid/_power_distributing/_component_managers/_pv_inverter_manager/_pv_inverter_manager.py | Added filtering to inverter_data stream and updated ComponentId import. |
| src/frequenz/sdk/microgrid/_power_distributing/_component_managers/_ev_charger_manager/_states.py | Updated import order for EVChargerData and ComponentId. |
| src/frequenz/sdk/microgrid/_power_distributing/_component_managers/_ev_charger_manager/_ev_charger_manager.py | Updated import order, wrapped merged EVCharger stream with filter_stream_events. |
| src/frequenz/sdk/microgrid/_power_distributing/_component_managers/_ev_charger_manager/_config.py | Updated ComponentId import. |
| src/frequenz/sdk/microgrid/_power_distributing/_component_managers/_component_manager.py | Updated ComponentId import. |
| src/frequenz/sdk/microgrid/_power_distributing/_component_managers/_battery_manager.py | Updated ComponentId import and applied filter_stream_events to data receivers. |
| src/frequenz/sdk/microgrid/_data_sourcing/microgrid_api_source.py | Updated ComponentId import. |
| src/frequenz/sdk/microgrid/_data_sourcing/_component_metric_request.py | Updated imports for ComponentId and ComponentMetricId. |
| src/frequenz/sdk/microgrid/_data_pipeline.py | Updated ComponentId import and removed unneeded imports. |
| pyproject.toml | Updated microgrid client dependency and frequencies channels version. |
| benchmarks/power_distribution/power_distributor.py | Updated ComponentId import to reflect new module path. |
| RELEASE_NOTES.md | Updated release notes to mention microgrid client dependency changes. |
src/frequenz/sdk/microgrid/_power_distributing/_component_managers/_battery_manager.py
Outdated
Show resolved
Hide resolved
b9cc2ff to
30b6d21
Compare
| filterwarnings = [ | ||
| "error", | ||
| "once::DeprecationWarning", | ||
| "once::PendingDeprecationWarning", | ||
| # We use a raw string (single quote) to avoid the need to escape special | ||
| # chars as this is a regex | ||
| 'ignore:Protobuf gencode version .*exactly one major version older.*:UserWarning', | ||
| ] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if you added this just for consistency or if you are also getting protobuf warnings at this level 😱
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am getting warnings indeed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Damn, I didn't know they were so intrusive...
30b6d21 to
faf2087
Compare
Signed-off-by: Mathias L. Baumann <[email protected]>
faf2087 to
a3de905
Compare
|
Test with nox (amd64, ubuntu-24.04, 3.13, pytest_min) is consistently timing out due to some infinite loop while cleaning up for tests (tests finish, but the reporting of warnings, etc. is never ending) 😟 |
|
Not sure if it might be related or a similar issue to the These are the logs after the tests finish(keeps looping on the last runtime error in " File "/home/runner/work/frequenz-sdk-python/frequenz-sdk-python/src/frequenz/sdk/microgrid/_power_distributing/_component_status/_battery_status_tracker.py", line 267"). |
We have no guarantee that the event loop is still running at the time the __del__ method is called. In fact we do see errors in the in the test cleanup phase exactly because of this. Signed-off-by: Mathias L. Baumann <[email protected]>
45c4ad7
| def __del__(self) -> None: | ||
| """Destroy this instance. | ||
| Cancel all running tasks spawned by this background service. | ||
| """ | ||
| self.cancel("{self!r} was deleted") | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the fix is to check if the event loop is running before calling cancel, because when the loop is there, we still want to cancel and cleanup.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought about that, but I don't think we gain much from that tbh. Cleanup should not rely on del
Blocked by:
The release of base-client v0.11The release of microgrid client